Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add LXC/LXD completions. #3554

Merged
merged 1 commit into from Nov 18, 2016
Merged

Conversation

haarts
Copy link
Contributor

@haarts haarts commented Nov 15, 2016

Description

This adds (part of) the completions for LXD.
At the moment this covers only the most basic use cases.
Texts are taken from lxc help command.

TODOs:

  • [x ] Changes to fish usage are reflected in user documenation/manpages. not required here
  • [x ] Tests have been added for regressions fixed. not required here

end

function __fish_lxc_list_containers
lxc list -c n | grep '| \w' | sed -e 's/\(|\| \)*//g'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer using our string tool for completions - string match -r '\| \w+' | string replace -r '\| ' ''?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splendid! Fixed.

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments monitor --description 'Monitor activity on the LXD server.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments move --description 'Move containers within or in between lxd instances.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments network --description 'Manage networks.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments pauze --description 'Changes state of one or more containers to pause.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: I assume it's "pause"? Also "changes state to X" sounds a bit weird - why not just "pauses container" and similar for the other states.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo indeed.

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments delete --description 'Delete containers or container snapshots.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments exec --description 'Execute the specified command in a container.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments file --description 'Manage files on a container.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments finger --description 'Fingers the LXD instance to check if it is up and working.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Check if the LXD instance is up"?

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments exec --description 'Execute the specified command in a container.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments file --description 'Manage files on a container.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments finger --description 'Fingers the LXD instance to check if it is up and working.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments help --description 'Usage: lxc [subcommand] [options]'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Print help"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only one I had to depart from the lxc help <command> approach. I'll change it.


complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments config --description 'Manage configuration.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments copy --description 'Copy containers within or in between lxd instances.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments delete --description 'Delete containers or container snapshots.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would just "containers or snapshots" be okay here?

Descriptions should be as short as possible to allow more columns to be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you that that text would be better. However, I took these texts directly from the lxc help <command> output. Leaving it would mean the original output is the One And Only Truth. I kinda like that too.
I'm a little on the fence here.

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments help --description 'Usage: lxc [subcommand] [options]'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments image --description 'Manipulate container images.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments info --description 'List information on LXD servers and containers.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments init --description 'Initialize a container from a particular image.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "particular" seems redundant.

complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments publish --description 'Publish containers as images.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments remote --description 'Manage remote LXD servers.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments restart --description 'Changes state of one or more containers to restart.'
complete --condition '__fish_lxc_no_subcommand' --command lxc --no-files --arguments restore --description 'Set the current state of a resource back to a snapshot.'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's "resource" here? Something different from a container? A superset?

At the moment this covers only the most basic use cases.
Texts are taken from `lxc help` command.
@faho
Copy link
Member

faho commented Nov 17, 2016

@haarts: I'll leave the wording decisions up to you. Let me know when you've decided.

@haarts
Copy link
Contributor Author

haarts commented Nov 18, 2016

@faho I'd like to echo the wording of LXD. But I agree with your suggestion. Lemme try to fix that upstream. In the meanwhile I think we should merge this. When upstream changes I'll create an other PR.

@haarts
Copy link
Contributor Author

haarts commented Nov 18, 2016

Upstream issue: https://github.com/lxc/lxd/issues/2627

@faho faho merged commit 36d4283 into fish-shell:master Nov 18, 2016
@faho
Copy link
Member

faho commented Nov 18, 2016

Merged, let's see what upstream thinks.

@haarts haarts mentioned this pull request Nov 18, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants